-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Backtrace 0.3.68 #113176
Backtrace 0.3.68 #113176
Conversation
|
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@bors r+ |
…=thomcc Backtrace 0.3.68 Uptakes these releases: - https://github.com/rust-lang/backtrace-rs/releases/tag/0.3.68 - https://github.com/rust-lang/flate2-rs/releases/tag/1.0.26 r? `@thomcc`
⌛ Testing commit 2150451 with merge 120598573092f2b9713914ed4e762758f9d81394... |
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
@rustbot author |
I have no idea what's wrong. |
2150451
to
3127e99
Compare
📌 Commit f9f5dbca6c3377a70dcbcd21bc7f4397dd879fa3 has been approved by It is now in the queue for this repository. |
@bors p=5 |
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
f9f5dbc
to
7fef07b
Compare
Dedup addr2line, miniz_oxide, object in .lock
7fef07b
to
079949d
Compare
☀️ Test successful - checks-actions |
Finished benchmarking commit (571c9fc): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 668.087s -> 667.216s (-0.13%) |
The only things I can think of in the backtrace crate which would so significantly alter performance would be the change to explicitly use mmap64, or the change to handle split dwarf. Otherwise I don't think it's directly related. I'll try to followup. |
It's a bit surprising since I wouldn't have expected backtraces to actually be computed in the compiler code that's relevant here. |
Cachegrind diffs (e.g. here) might be informative. |
Okay, that actually does clarify things, but specifically it eliminates any hypotheses I had about anything specifically in the backtrace mattering, because I can't see anything from the backtrace crate active there, yet linking takes longer. So I think it's just about binary size. Because backtrace goes into every binary, even slightly more to compile can be a hard hit. I think this time it might have been unavoidable, because if we're going to do a backtrace, and if we're going to use debug formats, we really want to be able to symbolicate with out-of-line data like split DWARF for... more or less the same reason that this was a perf hit. But I am going to keep an eye out for opportunities to reduce code size in that crate. |
I have opened rust-lang/backtrace-rs#541 as a first step towards addressing this in the future. Backtrace has probably been gradually racking up bloat over years. It may have good reasons to add more to our binary size in the future! So this should probably be treated as more of an ongoing problem that we need to attack with ongoing measures. |
Uptakes these releases:
r? @thomcc